feat: factor refactor D4b — stream minute materialization per symbol - #97
Merged
StackOverFlow11 merged 10 commits intoJul 25, 2026
Merged
Conversation
D4's materializer loaded every symbol's 1min bars into ONE frame. On the real
evaluation plane that frame is 52.7 GB nominal and ~115 GB peak RSS against
56 GB of memory, so the D5 four-leg reconciliation could not be run at all --
not slowly, at all. It also affected BOUNDED factors, not just pooled ones, so
per-symbol streaming is a structural prerequisite for all eleven minute
factors rather than an optimization for eight of them.
The materializer now reads one symbol's bars, reduces them to that symbol's
daily intermediate, discards the bars, and runs the factor's cross-sectional
combine ONCE on the assembled panel.
THE CUT SITS BEFORE THE CROSS-SECTION. Ten minute factors are per-symbol pure,
so their intermediate is their value and the combine is the identity.
intraday_amp_cut is not: its step-4 z-score needs >= AMP_CUT_MIN_CROSS_SECTION
(=10) finite pairs on a date, so a one-symbol cross-section is all-NaN by
definition. Splitting around the WHOLE factor would silently zero it out; the
split before its combine reproduces it exactly. That constant is untouched --
no factor math, parameter or threshold changes here. The seam already existed
in the factor module and in the eval runners; the binding surfaces it.
Saturation is now decided PER SYMBOL, which is the same terminal, not a
weakening: the criterion's whole content is "further history cannot change any
emit value", so a symbol satisfying it at depth D has the values it would have
at any deeper load. The locked head is dropped from the symbol's own day list
rather than the loaded union, and the union is a superset, so the per-symbol
window is at or inside the union one -- it never terminates earlier. The
requested-symbol veto, the locking offset, the provider's declared floor and
the loud exhaustion raise are all unchanged; what a thin symbol can no longer
do is drag the other 994 to the floor (measured: 84 of the evaluation
universe's 995 symbols list after the emit start, so that drag was
constructive for every pooled factor).
Two disclosed consequences:
* PER-SYMBOL TRIM: the trailing-trading-day trim reads the symbol's own trading
days instead of the loaded universe's union. Strictly more conservative (a
subset reaches at least as far back) and it removes a cross-symbol coupling
that was itself a load-geometry dependence -- another symbol's calendar could
decide how much history this one was warmed with. Pinned by a test that first
asserts the two rules really disagree on its fixture.
* The engine RE-FILTERS what a provider returns. Streaming makes correctness
depend on the provider honouring its symbols argument; a provider ignoring it
would emit every other symbol's rows once per streamed symbol. The engine owns
that invariant rather than trusting an injected object with it.
test_zero_output_symbol_veto_mutation keeps its fixture, factor and assertions
but re-expresses its mutation. The defect it encodes is unchanged ("a symbol
that produced nothing gets no veto"); its old wording ("restrict the criterion's
symbol list to symbols present in the output") was faithful only while one
criterion call decided the depth for the whole universe. Measured under
streaming: 70 criterion calls, real and filtered forms answer identically in
70 of 70, because all 20 zero-output calls are already vetoed by the guard
above the symbol loop. Left alone it would be a mutation test that cannot fail.
Re-expressed directly it reproduces the original divergence on the same
fixture: single-fill 40 rows / one symbol vs batch 50 rows / two symbols.
qt.saturation_probe gains --mode stream-scale: the same instrument pointed at
the fixed engine, one factor per process (so ru_maxrss is that factor's own
peak) and resumable via a JSONL out file. It reports the nominal largest frame
AND the peak RSS separately, because the feasibility mode measured
frame:peakRSS at 2.2-2.45x and the nominal number alone understates the
requirement by about half.
…nger uses D4b moved the materializer from a whole-universe single frame to per-symbol streaming. Two tools still described the old shape, which is the exact failure mode this repo has paid for three times (#76, #78, #82): the behaviour changed and the sentence about it did not. * qt/saturation_probe feasibility mode measures the PRE-D4b engine. Its item 3 said "the materializer loads every symbol into ONE frame", present tense. It is now marked HISTORICAL -- it is what that geometry would have cost, i.e. the reason it was replaced, not what runs today -- and the module header points at --mode stream-scale for the current path. Items 1, 2 and 4 are facts about the DATA, not the engine, so they stand as written. * qt/factor_hotpath_smoke loads its 40-symbol sample into one frame in BOTH arms. What it measures is unchanged and still the point (repeated 1min read + normalize vs paying it once), but the NAIVE arm is no longer a picture of the engine's memory profile, and a reader could reasonably take it for one. Said so, with a pointer to the tool that does measure the real path. Deliberately NOT swept: the eleven eval runners' "the multi-year all-symbol minute panel is NEVER materialized". Those describe the runners, which still stream per symbol -- a careless sed here would turn true statements false, which is the trap #82 called out by name.
Both tests could have agreed for the wrong reason if their fixture drifted, and neither would have said so: * the early-saturation test borrowed "the dense symbols stop short of the floor" from the test above it, in prose. On a shorter fixture the first expansion chunk would already reach the floor, both sides would load the same bars, and the comparison would pass while exercising nothing. It now records the load starts itself and asserts the early stop. * the cross-section test compared the streamed values against the one-frame reference with an extra universe member present. If that member changed nothing, agreement would be equally consistent with a per-symbol cross-section -- exactly the failure it is meant to exclude. It now asserts that the extra member actually moved existing cells (measured: it does).
…seline The feasibility probe's section 5 left "per-symbol saturation: implemented and accepted" open, and said explicitly that it had NOT reconciled intraday_amp_cut end to end. Section 6 closes that in the same file, so the before and the after sit next to each other instead of in two documents that can drift apart. Recorded, all measured: * cell-by-cell reconciliation on REAL bars (40 symbols x 6 months) against the geometry that was replaced -- every symbol in one frame. Index identical and NaN-set difference ZERO on all ten factors; five bit-identical, the other five within 3.3e-14 (pandas rolling accumulation over a differently-sized prefix, the residual D2/D4 already catalogued). The NaN column is the load-bearing one: a finite<->NaN flip is a coverage change, not a float artefact. * the scale proof on the real evaluation plane for the riskiest factor (intraday_amp_cut, the only pooled x cross-sectional one): nominal largest frame 0.0656 GB, PEAK RSS 0.964 GB against 56 GB available, 995 of 995 symbols present, live_calls 0. The two numbers are reported separately because the probe measured frame:peakRSS at 2.2-2.45x and the nominal figure alone understates the requirement by about half. * where the cost actually falls: a dense symbol saturates in ONE provider call (0.59 s), a late-listing symbol takes 40 (14.95 s) because it re-reads its own history on every expansion chunk. 40 is exactly the chunk arithmetic. That cost now lands on those 84 symbols instead of dragging all 995 -- which is the point, since the all-995 version did not fit in memory at all. Also disclosed: wall times are upper bounds (other acceptance jobs were running on the same box); the memory figures are per-process high-water marks and are not affected. And the expansion STEP SIZE is a documented non-correctness knob that could cut those 40 reads, but changing it is still a load-geometry change that owes its own reconciliation, so it is left to D5 rather than bundled here.
CROSS_SECTIONAL_MINUTE_FACTORS was a declaration with a comment explaining it. The comment is true today; nothing made it stay true. A minute factor that later grew a cross-sectional step without being added to the set would be streamed with an identity combine and would silently return per-symbol values -- the same class of defect as splitting around the whole factor, which this step exists to prevent. The classification is now derived and asserted on every bound factor, in both directions: a factor is per-symbol pure iff splitting around the WHOLE factor reproduces the whole-universe values. Nine must survive that split bit-identically and intraday_amp_cut must not. Both halves are non-vacuous (the whole-universe side is asserted to have finite values first).
…tries One factor per load geometry, on the real evaluation plane, one process each: * intraday_amp_cut_10 (pooled x CROSS-SECTIONAL) peak RSS 0.964 GB, 1990.5 s * ridge_minute_return_20 (pooled) peak RSS 0.854 GB, 1133.7 s * jump_amount_corr_20 (bounded) peak RSS 0.730 GB, 417.7 s That is complete coverage of the geometries, not a sample of the factors: every minute factor takes one of these three paths. All three carry 995 of 995 symbols and live_calls 0. The peak sits in a 0.73-0.96 GB band regardless of factor or output size, because it is set by one symbol's bars plus the daily panel and not by the universe -- which is the property that makes the run possible at all, against the 52.7 GB nominal / ~115 GB peak the replaced geometry needed. Wall time orders exactly by geometry: no expansion < 20-chunk expansion < 60-day-chunk expansion. The remaining seven factors are deliberately not run here: that is the D5 full run, not D4b's acceptance. The sweep skips factors already in its JSONL (verified by running it again against a recorded factor), so D5 continues rather than restarts.
…ED-1)
Review found the one real engine defect D4b introduced. The single-frame engine
was immune to a repeated symbol because the providers' isin filter is idempotent
-- whatever the caller passed, one row per name came back. Streaming iterates the
list, so a repeat is materialized twice.
Measured on the streaming fixture with two names repeated, before the fix:
volume_peak_count_20 48 -> 56 rows, 8 duplicated (date, symbol) entries,
values unchanged (the factor is per-symbol pure)
intraday_amp_cut_10 the same 8 duplicates AND ALL 48 shared cells changed
The second one is the serious half: a factor with a real cross-sectional combine
sees the repeated name twice in its date's cross-section, which moves that date's
mean and std for EVERY member. FactorValueStore.upsert's duplicated(keep="last")
would de-duplicate the ROWS while keeping the contaminated VALUES, so it would
land in a persistent artifact. No live path can reach this today (the service is
not wired to a runner until D6), which is why it is not a HIGH -- but the mirror
of it was already taken seriously: the engine re-filters what a PROVIDER returns
and explains that it owns that invariant rather than trusting an injected object.
The caller-side half of the same invariant was not owned. Now it is, in one place
at the engine's entry point, covering the daily path too.
DE-DUPLICATING RATHER THAN RAISING, and why: it reproduces exactly what the
single-frame engine returned for the same call, which keeps D4b a change of
memory profile and nothing else. A raise would be a new failure mode for input
the previous engine accepted -- a behaviour change in the opposite direction, and
one that belongs to whoever validates a universe, not to the value engine.
MUTATION (run): removing the call restores 56 rows / 8 duplicates / 48 changed
cells (probe asserted BEFORE trusting the test), test rc=1; restored rc=0.
… the trim test Three review items, all in tests. MED-2 -- the JC5 docstring stated its own reasoning backwards. It said leaving the old mutation expression in place "would be a mutation test that cannot fail". It would not: the assertion asserts that the drop HAPPENS, so an inert mutation makes it impossible to SATISFY. The old expression went RED on the D4b engine; it was a test that cannot pass. That distinction is not pedantic here -- the docstring was invoking this project's heaviest methodology rule while describing its exact opposite, next to a test whose real situation is the other rule (do not quietly edit a test the new engine fails). Restated honestly, and the call count corrected from 70 to 72: the earlier figure counted only the per-date fill and missed the batch fill's 2 calls. Re-measured: 72 calls, 0 divergences, 20 zero-output calls, all of them already vetoed by the guard above the symbol loop. LOW-4 -- the per-symbol trim test covered only jump_amount_corr_20, whose disagreement with the union rule shows up as a CHANGED VALUE. The other two bounded factors express the same disagreement as a COVERAGE FLIP (NaN under the union rule, finite under per-symbol) -- and finite<->NaN is precisely what _assert_bit_identical singles out as the change this step must not make by accident. Testing only the value-moving one missed the two that matter most. Now parametrized over all three, derived. Also recorded there, because it reverses this file's earlier framing: the trim is not a conservative refinement of a correct rule, it CORRECTS a wrong one. Measured across engines, all three bounded factors move exactly one cell and in all three the per-symbol result equals the load-geometry-free truth while the union result does not (jump_amount_corr 0.157115 union vs 0.090115 truth; the other two NaN vs finite). LOW-5 -- STREAMED_FACTOR_IDS was a hand-maintained tuple with nothing tying it to the binding tables. A new minute factor could enter both tables, satisfy the coverage test, and still slip past every parametrized reconciliation silently. Derived from the binding table now, with the id round-tripped through the registry so the derivation itself is checked; BOUNDED_FACTOR_IDS likewise.
…ee doc claims NIT-6 -- --mode stream-scale reproduced the scale table, but NOTHING in the repo reproduced section 6.1, the most load-bearing table in the document: a reviewer could only re-derive it by writing their own script. The probe is already the project's "committed, re-runnable instrument"; this holds it to that standard. --mode reconcile runs the 40-symbol x 6-month comparison against the whole-universe single frame and exits rc=1 if any factor's index or NaN set differs. Its output is now transcribed into 6.1 literally, same columns and same order, so the two can be diffed directly. Verified: the committed mode reproduces every number. NIT-7 -- 6.1 never stated the reference's load depth, which is load-bearing: the streamed side's depth is decided by saturation expansion for the 8 pooled factors, so a shallower reference compares two different histories and a deeper one changes the float-accumulation prefix. Stated (400 calendar days), with the reason, and it explains why this table shows ~1e-14 residuals while a branch-vs-main comparison of the real engine gets exactly 0.000e+00 -- there both sides load identically. MED-3 -- 6.1 declared a policy this step does not actually keep: that finite<->NaN flips are "never allowed here". per-symbol trim CAN produce them and measurably does (two of three bounded factors); the 40-symbol sample simply has no symbol sparse enough to trigger it. Corrected to say what is true: zero flips ON THIS SAMPLE, with the one disclosed semantic change that can produce them named. Direction reversal (new 6.5) -- this document warned that if D5 mismatches the D1 frozen baseline on sparse symbols, the trim is the first place to look. That is backwards, on the safe side. Cross-engine measurement, three bounded factors, one cell each: per-symbol == the load-geometry-free truth in all three, union != truth in all three. The union rule was not looser, it was WRONG (red line #6: a value that depends on load geometry). And the eleven eval runners read each symbol's whole window with no union trim at all, so per-symbol moves TOWARD the frozen baseline, not away. The warning is restated pointing at saturation depth and anchor truncation, which are the named attribution items.
…tor must fail Deriving the parametrize lists from the binding tables only guarded factors ADDED to a table. Review measured the open direction: dropping one factor from both tables left this file at 53 passed and the suite at 2169 passed, all green, with the factor silently absent from every reconciliation. The source of truth is now the set of Factor subclasses defined under factors/compute/minute, so a new factor file that nobody classifies fails here. Mutation witness: removing PeakIntervalKurtosisFactor from both tables (occurrences 4 -> 0, asserted before running) turns this test red; restoring makes binding.py byte-identical and the file green at 58.
StackOverFlow11
added a commit
that referenced
this pull request
Jul 25, 2026
docs: record refactor step D4b (PR #97)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Factor-layer refactor step D4b (implementation-time amendment A1 to design v3.2): make the minute materializer stream per symbol — read one symbol's bars, aggregate to its daily factor series, discard — with the seam placed before the cross-sectional combine. This is a factor values-path change, so it carries its own four-leg acceptance rather than riding along inside D5.
Why this step exists
D5's opening probe found that D4's materializer cannot run the evaluation universe at all: it loads the whole universe in a single frame (
materialize.py:262), and at CSI500 × 5 years that is ~2.9 × 10⁸ rows — nominal 52.7 GB for the eval window, 122.2 GB at floor depth, with measured peak RSS at 2.2–2.45× the frame (per-symbol parts and the concatenated result coexist), i.e. a true peak around 115 GB against 56 GB available. 84 of 995 symbols have their first 1min bar at or afteremit_start, so pooled saturation can never terminate early and always expands to the floor — D4's disclosed cost turns out to be structurally guaranteed at full scale. Three cheaper escapes were evaluated and refuted (details in PR #95 and in the design doc's amendment A1);intraday_amp_cutcloses the last door by being both pooled and cross-sectional, so per-symbol batching destroys its cross-section while time-chunking cannot bound its depth.Worth stating plainly: the old runners' docstrings say outright that "the multi-year all-symbol minute panel is NEVER materialized". D4's load geometry was a regression relative to what it replaces — invisible at D4's 40-symbol acceptance scale. The design's §8 sized the decision hot path (a 20-day tail) and never sized the evaluation path, which is why nobody caught it at design review.
Scale: the point of the whole step
Measured on the real evaluation universe (CSI500, 995 symbols × 5 years, cache-only,
stk_mins_live_calls=0) for one factor of each of the three load geometries — pooled×cross-sectional, pooled, bounded — all three at 995/995 symbols, 1 symbol per provider call. Peak stays at 0.73–0.96 GB regardless of factor or output size: it is set by "one symbol's bars + the daily panel", not by the universe. The cost of expanding to the floor now falls only on the 84 late-listing symbols (40 provider calls / 14.95 s each) instead of dragging the other 911 along (dense symbols: 1.0 call / 0.59 s).Four-leg acceptance
factors/materialize.pyfrom that file alone, with the new per-symbol helpers covered), not passing by bypass.sys.modulescontains nofactors/qt/data/runtime/alpha(653 modules, 0 hits).volume_peak_count9/9 exactly 0.0;intraday_amp_cut≤ 1.998e-15.intraday_amp_cutagrees on 4,720 real cells. Reviewer re-ran it against main's engine as reference and got max|abs| = 0.000e+00 (cleaner than the doc's 2.531e-14 because both sides then share the load geometry — now explained in the doc).qt.saturation_probe --mode stream-scale, and the reconciliation table via the new--mode reconcile(added because the reviewer could not otherwise re-run the document's most load-bearing table from the repo).What the review changed (APPROVE-WITH-NITS → all closed)
The headline reclassification: the implementer described per-symbol trailing trim as "a more conservative semantic refinement". The reviewer proved it is fixing a real D4 defect. Same sparse fixture through both engines, only the trim differing:
jump_amount_corr_20minute_ideal_amp_10amp_marginal_anomaly_vol_20All three: D4b equals truth, D4 does not. Union trim was feeding sparse symbols fewer than
lookback_depthof their own trading days — the load-geometry dependence red line #6 exists to forbid. And the four old runners read per-symbol over the full window with no union trim at all, so per-symbol trim moves toward the D1 frozen baseline. The implementer's warning to D5 ("if a sparse-symbol cell mismatches the frozen baseline, look here first") had the direction backwards; it is corrected in the doc (new §6.5) and now points at saturation depth / anchor truncation instead. The superset property was proved rigorously (a subset's k-th largest ≤ the superset's), with no counterexample constructible.MED-1 (the one real engine defect, introduced by D4b): a duplicated symbol in the request was processed once per occurrence — 48 → 52 rows with 4 duplicate
(date, symbol)rows, and forintraday_amp_cutthe duplicate entered the cross-section twice and moved all 48 shared cells. D4 was immune because the provider'sisinabsorbs duplicates. Fixed by order-preserving de-duplication at thematerialize_rangeentry (not a loud raise: de-duplication reproduces what the single-frame engine returned for the same call, keeping D4b "changes the memory profile and nothing else", whereas raising would newly reject input the previous engine accepted). Reviewer verified cross-engine: the de-duplicated branch now matches main cell-for-cell, 0 differences on both factors.MED-2: a docstring claimed the replaced mutation expression "cannot fail"; run as-is on the D4b engine it fails — it is a test that cannot pass. Substance was fine (the property survives, fixture/factor/assertions untouched), but the wording invoked this project's heaviest methodology rule backwards. Reworded, and the call count corrected 70 → 72 (70 per-date + 2 batch; the original counted one arm).
MED-3: the feasibility doc declared a policy ("finite↔NaN flips are not allowed in this step") that per-symbol trim can and does violate — the 40-symbol sample simply lacked a sparse enough symbol. Replaced with what the sample actually shows.
LOW-4/5, NIT-6/7 all closed: the trim test now parametrizes over all three bounded factors (including the two whose divergence is a coverage flip); the factor-id lists derive from the binding tables with a registry round-trip;
--mode reconcilemakes §6.1 reproducible (reviewer ran it — 10 rows × 4 columns match the document exactly, and it is not a self-comparison: the reference side never touchesmaterialize_range); the reference's 400-calendar-day depth is now stated.Lead follow-up (this PR): the derived lists closed only one direction. Review measured the other — dropping a factor from both binding tables left the file at 53 passed and the suite at 2169 passed, all green, with that factor silently absent from every reconciliation. Added a closure test whose source of truth is the set of
Factorsubclasses defined underfactors/compute/minute, so an unclassified factor file fails. Mutation witness: removingPeakIntervalKurtosisFactorfrom both tables (occurrences 4 → 0, asserted before running) turns it red; restoring leavesbinding.pybyte-identical and the file green at 58.Gates (lead, independent)
pytest 2174 passed / 1 skipped (2117 before D4b + 57 new; node-ID set comparison shows 0 existing tests displaced) · ruff clean · phase0 anchor
ic 0.9600 / annual 0.8408unchanged · validate-config 31/31 · secret scan 0 ·exec_baseline_freeze --verify77/77 with head still45c14aa·artifacts/refactor_baseline/{panels,panels_d2}mtimes untouched ·factors/service.pydiff is 0 lines (A1's named trap — batching in the service layer, where_ensure_coveragecomputes gaps by date only — was not stepped in) · worktree clean.Disclosed limits / notes for D5
CACHE_MINUTE_DATA_START), not derived per symbol from the minute coverage ledger — the probe explicitly flagged that as a separate, definition-adjacent concern.valley_price_quantilestays DEFERRED (still needs the daily panel); D5's binding inherits the streaming path and must not bypass it with a fixed-depth trim.No CLAUDE.md/AGENTS.md changes here (docs PR follows). Frozen baselines untouched.